-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: PotentialFrame is parametric #377
base: main
Are you sure you want to change the base?
Conversation
a54bffb
to
0f959ca
Compare
0f959ca
to
2e236a7
Compare
e83bda0
to
3c47fbe
Compare
Signed-off-by: nstarman <[email protected]>
Signed-off-by: nstarman <[email protected]>
3c47fbe
to
c0b126a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #377 +/- ##
===========================================
- Coverage 96.59% 80.09% -16.50%
===========================================
Files 76 76
Lines 3028 3035 +7
===========================================
- Hits 2925 2431 -494
- Misses 103 604 +501 ☔ View full report in Codecov by Sentry. |
Hey @nstarman! I think the problem is indeed with generics. If I'm not mistaken, this reproduces the issue: In [8]: from typing import Generic, TypeVar
In [9]: T = TypeVar("T")
In [10]: class A(Generic[T]):
...: pass
...:
In [11]: resolve_type_hint(A)
Out[11]: __main__.A
In [12]: resolve_type_hint(A[int])
<ipython-input-12-9cf80670f4aa>:1: UserWarning: Could not resolve the type hint of `__main__.A[int]`. I have ended the resolution here to not make your code break, but some types might not be working correctly. Please open an issue at https://github.com/wesselb/plum.
resolve_type_hint(A[int])
Out[12]: __main__.A[int] It should fortunately be fairly easy to fix this. This I don't quite have the time to fix this now (sorry :( ), but could possibly at the end of this week or the beginning of next week! EDIT: I should add that Plum doesn't support |
Thanks @wesselb for investigating the issue! |
Ah, I replied without thinking this through too carefully. Plum currently does not support generics, but it could certainly support them in the future. The fact that you cannot construct instances is unimportant! There are two ways to support generics: (1) if I think an incomplete implementation supporting commonly used patterns of generics should actually be feasible, although technical.
If you could, then that would be amazing. :) This should solve the warning. |
This PR makes
PotentialFrame
parametric wrt to the potential type.